-
Notifications
You must be signed in to change notification settings - Fork 459
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add kernelCTF CVE-2023-6931_lts_cos #135
base: master
Are you sure you want to change the base?
Conversation
933028d
to
c111d81
Compare
76d6b57
to
bf0d68f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey,
This is just a quick code quality review. We're planning to review the submissions more deeply (actually understanding what the exploit does) in two weeks.
In general, the code quality looks great, some of the calculations are hard to understand but I think they are explained well enough in the writeup. I've left two comments about improvement ideas.
We also have a draft style guide now: https://google.github.io/security-research/kernelctf/style_guide. Although your exploit code is usually already high quality, so you probably don't have to depend on it too much, but let us know if you have any feedback regarding the style guide.
Thanks for the submission and PR!
#define TFD1 ((READ_BUF_SIZE - 8 + OOB_LOC1)/8) | ||
#define TFD2 ((READ_BUF_SIZE - 8 + OOB_LOC2)/8) | ||
/* Number of increments */ | ||
#define NUM_INCS1 0x830 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If possible, then I think it would make it clearer what's going on if you would #define
the kernel address for sk_destruct
and sk_write_space
and then use #define NUM_INCS1 (SK_DESTRUCT - SK_WRITE_SPACE)
, maybe with a static assert that NUM_INCS1 > 0
.
Similarly for NUM_INCS2
you can add #define
s for netlink_bind
and netlink_bind_ret_addr
.
This helps porting the exploit from one target to another.
|
||
rop += (JOP_OFF - SK_DESTRUCT_OFF)/8; | ||
/* commit_creds(prepare_kernel_cred(0)) */ | ||
rop++; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This rop++
is still part fo the stack pivot based of pop rbp
and not related to commit_creds
, right? Maybe move one line up (before commit_creds
) and add a comment that this line is for skipping pop rbp
?
No description provided.